This links to my comment here onflow/flips#331 (comment).

You are adding inclusion fees here, so you do not need to have a minimum execution effort limit. I would have the inclusion effort configurable though.

I don't think it makes sense to have a configurable inclusion fee because that would require us to predict network load for the future, which isn't possible, right? We'd have to predict it for each timestamp. Additionally, the fee multipliers are already configurable, so that kind of solves the same problem as a configurable inclusion fee. I also responded to your comment on the FLIP about the minimum effort

the fees include the storageFee but the since its still in the CallbackData it doesn't actually help the account holding this data with storage capacity. Is this ok? do we just plan to have a high enough balance on the account for this to not be a problem?

that is a good point. I think that we'll have enough FLOW in the account that we won't have to worry about it, but we still should probably deposit those extra storage fees to the account's flow vault.

Now that I am thinking about it, I kind of want to just deposit all fees to the account's vault temporarily instead of storing them in CallbackData. It simplifies the contract but it does create a bigger honeypot that would be worse if a vulnerability was found. How does that sound?

I don't think this is a big concern, probably we ran out of computation before storage.

@bluesign what do you mean? like the computation of storing a large piece of data is more costly than the actual storage fees?

yeah I think adding ( writing ) some bytes ( considering it is small ) to a large dictionary ( where we may have hypothetical problem ) will be more cheaper in storage fees vs computation required.

It is pretty hard to predict as we don't have fees technically, and luckily no-one is abusing.

can we try doing this without recursion? Recursion is costly.

How costly is recursion? I don't see this ever getting to more than a few levels deep and this is a clean way to do it

I think stack depth is 2000, cost to authorizer is negligible ( though to system it can have heavier cost )

With recursion the memory cost is a lot higher which doesn't really translate to fees, but could halt the transaction. the execution cost is also higher, but not by that much, I cant really give you any concrete numbers at this time.

If you feel its cleaner this way we can keep it if the stack isn't going to be too deep most of the time.

The size of Path is not any more bounded than e.g. a String so seems counterintuitive to round it to 0.0 regardless of length.

See e.g.:

transaction {
    execute {
        var stringToStore = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
        log("Length of string: ".concat(stringToStore.length.toString()))
        log(
            "getSizeOfData if stored as string:".concat(
                FlowCallbackScheduler.getSizeOfData(
                    stringToStore
                ).toString()
            )
        )
        log(
            "getSizeOfData if masked as path:".concat(
                FlowCallbackScheduler.getSizeOfData(
                    StoragePath(identifier: stringToStore)
                ).toString()
            )
        )
    }
}

Produces:

6:51AM INF LOG: "Length of string: 510"     
6:51AM INF LOG: "getSizeOfData if stored as string:0.00057500"
6:51AM INF LOG: "getSizeOfData if masked as path:0.00000000"

good point. I didn't consider that! I'll remove Path from the list

The FLIP says that a low priority callback "is executed opportunistically, only in blocks after the scheduled timestamp".

However, it seems this code may process low-priority callbacks before they are due - lowPriorityCallbacks is a collection of all low-priority callbacks as they are all stored at the special timeslot 0.

oh man, I can't believe I didn't have a test for that. Good catch!

nit: FLIP says "The provided execution effort value must be bigger than 10". Even though this is configurable, would IMO make sense to align the default value and the value mentioned in the FLIP.

good point. We'll update that

thanks, I'll update that